Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ supervisor: implement support for autoscale to/from zero #3161

Merged

Conversation

chrischdi
Copy link
Member

@chrischdi chrischdi commented Aug 19, 2024

What this PR does / why we need it:

  • Adds a reconciler for vmwarev1 VSphereMachineTemplate
  • Adds status subresource for vmwarev1 VSphereMachineTemplate for autoscaler resources
  • Implements support for setting CPU and Memory in .status for vmwarev1 VSphereMachineTemplate from the referenced VirtualMachineClasses

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #2995

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 19, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 19, 2024
main.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch from a8a162c to d1752a2 Compare August 20, 2024 15:18
Copy link
Member

@sbueringer sbueringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't have the time today to also review the unit tests, will do that tomorrow

controllers/vmware/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vmware/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vmware/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
controllers/vmware/vspheremachinetemplate_controller.go Outdated Show resolved Hide resolved
Copy link
Member

@sbueringer sbueringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall nice, just a few nits here and there

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 22, 2024
@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch from 98570bb to 887298a Compare August 22, 2024 06:45
@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch 2 times, most recently from 99ae2cc to 726c7f7 Compare August 23, 2024 11:56
@sbueringer sbueringer mentioned this pull request Aug 26, 2024
16 tasks
Copy link
Member

@sbueringer sbueringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

test/go.mod Outdated Show resolved Hide resolved
test/e2e/quick_start_test.go Outdated Show resolved Hide resolved
test/e2e/autoscaler_test.go Outdated Show resolved Hide resolved
Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

test/e2e/config/vsphere.yaml Show resolved Hide resolved
@sbueringer
Copy link
Member

/cherry-pick release-1.11

@k8s-infra-cherrypick-robot

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch from 726c7f7 to 3b02d29 Compare August 26, 2024 11:51
@chrischdi
Copy link
Member Author

/test help

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
  • /test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main
  • /test pull-cluster-api-provider-vsphere-test-main
  • /test pull-cluster-api-provider-vsphere-verify-main

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-vsphere-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-vsphere-apidiff-main
  • pull-cluster-api-provider-vsphere-test-main
  • pull-cluster-api-provider-vsphere-verify-main

In response to this:

/test help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@chrischdi
Copy link
Member Author

/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main

@chrischdi chrischdi changed the title ✨ [WIP] supervisor: implement support for autoscale to/from zero ✨ supervisor: implement support for autoscale to/from zero Aug 26, 2024
@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch from 3b02d29 to 8a0f30c Compare August 26, 2024 12:02
@chrischdi
Copy link
Member Author

/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main

@sbueringer
Copy link
Member

Thx!

/lgtm
/approve

/hold
for tests

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2024
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: a82d724b11768607f76b22e6e7772964b907db12

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2024
@chrischdi chrischdi force-pushed the pr-vspheremachintetpl-autoscaler branch from 8a0f30c to ce106be Compare August 26, 2024 14:13
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2024
@chrischdi
Copy link
Member Author

/test pull-cluster-api-provider-vsphere-e2e-vcsim-govmomi-main
/test pull-cluster-api-provider-vsphere-e2e-vcsim-supervisor-main

@sbueringer
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 26, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 786975116044f8fd981aa946f4a34f9ca9aff252

@sbueringer
Copy link
Member

@chrischdi Ready to merge? :)

@chrischdi
Copy link
Member Author

Yes, tests all green

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2024
@k8s-ci-robot k8s-ci-robot merged commit 1b7c2dc into kubernetes-sigs:main Aug 26, 2024
14 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Aug 26, 2024
@k8s-infra-cherrypick-robot

@sbueringer: new pull request created: #3171

In response to this:

/cherry-pick release-1.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants